Skip to content

Enforce PKCE metadata compliance in OAuth client (#730)#1700

Open
PranavSenthilnathan wants to merge 2 commits into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan/fix-pkce-metadata-compliance
Open

Enforce PKCE metadata compliance in OAuth client (#730)#1700
PranavSenthilnathan wants to merge 2 commits into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan/fix-pkce-metadata-compliance

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Contributor

Summary

Fixes #730.

The OAuth client was defaulting code_challenge_methods_supported to ["S256"] when it was absent from authorization server metadata:

metadata.CodeChallengeMethodsSupported ??= ["S256"];

This masked non-compliant authorization servers. Per the MCP 2025-11-25 spec (and the current draft), clients MUST verify PKCE support via authorization server metadata and refuse to proceed if code_challenge_methods_supported is absent or doesn't advertise S256.

Changes

  • ClientOAuthProvider.GetAuthServerMetadataAsync: removed the ["S256"] default. The discovery loop now validates PKCE support per endpoint. If a well-known document omits code_challenge_methods_supported or lacks S256, that endpoint is skipped and discovery falls through to the next one — different discovery mechanisms (OAuth 2.0 Authorization Server Metadata vs. OpenID Connect Discovery) can return different documents for the same server, and OIDC metadata commonly includes the field. Discovery fails only once all endpoints are exhausted, consistent with the existing authorization_endpoint fall-through behavior.
    • The PKCE check lives in its own try block with a dedicated LogAuthServerMetadataMissingPkceSupport warning, distinct from the generic metadata-fetch error log.
  • Back-compat preserved: the 2025-03-26 fallback path (BuildDefaultAuthServerMetadata, used when no protected-resource metadata is available) still synthesizes S256, so legacy servers without metadata discovery continue to work.

Tests

  • CannotAuthenticate_WhenMetadataOmitsPkceMethods — metadata with no code_challenge_methods_supported is rejected.
  • CannotAuthenticate_WhenMetadataLacksS256PkceMethod — metadata advertising only plain is rejected.
  • CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt — when the OAuth 2.0 endpoint omits PKCE but the OpenID Connect document advertises it, the client falls through and authenticates successfully.
  • Test OAuth server gained a CodeChallengeMethodsSupported property and a MetadataPathsWithoutPkceSupport set to simulate endpoints with differing PKCE advertisement.

All 46 OAuth AuthTests pass on net8.0/net9.0/net10.0.

Spec note

PKCE metadata verification was introduced in the 2025-11-25 spec; 2025-03-26 and 2025-06-18 required PKCE usage but not metadata verification. The current draft keeps the 2025-11-25 language unchanged.

…l#730)

The OAuth client defaulted code_challenge_methods_supported to S256 when absent
from authorization server metadata, masking non-compliant servers. Per the MCP
2025-11-25 spec, clients MUST verify PKCE support via metadata and refuse to
proceed without it.

Discovery now skips any well-known endpoint whose metadata omits PKCE support or
lacks S256, falling through to the next endpoint (OAuth 2.0 vs OpenID Connect
documents can differ), and fails only once all endpoints are exhausted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
{
LogErrorFetchingAuthServerMetadata(ex, wellKnownEndpoint);
LogAuthServerMetadataMissingPkceSupport(ex, wellKnownEndpoint);
continue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If PRM is unavailable, don't we still reach BuildDefaultAuthServerMetadata here and synthesize S256? That would let metadata without code_challenge_methods_supported proceed through the legacy fallback. Can we limit that fallback to cases where no metadata document was found?

Previously, when PRM was unavailable, a discovered auth server metadata
document that lacked 'code_challenge_methods_supported' would be skipped
and the client would synthesize default metadata advertising S256,
letting non-compliant servers proceed. Now the legacy default fallback
only applies when no metadata document is discovered; a document that
fails PKCE validation surfaces the specific failure instead.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 57bac4ff-fb62-4b33-88c3-342ce65e9d0a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDK Implementation of Authorization is not compatible with the specification. code_challenge_methods_supported is handled incorrectly

3 participants